Refactor BaseMetricService (#17) by sonnenco · Pull Request #37 · deyna256/betterforces

sonnenco · 2026-02-15T18:21:51Z

Issue

Changes

Refactored /backend/domain/services/base.py from BaseMetricService class with static methods to standalone functions.
Removed import and use of BaseMetricService class from backend/domain/services/abandoned_problems_service.py due to lack of invocation of _deduplicate_problems or _filter_successful_submissions.
Updated the import statements and use of BaseMetricService in several files to point to the standalone functions, including:

* backend/domain/services/tags_service.py
* backend/domain/services/difficulty_distribution.py
* backend/domain/services/__init__.py
* backend/tests/unit/base_metric_service/test_deduplicate_problems.py
* backend/tests/unit/base_metric_service/test_filter_successful_submissions.py

Testing

The following tests come out identical to the main branch:

Backend: linting, type checking, tests
Frontend: linting, type checking, build

Please let me know if there are any questions or concerns.

…ass with static methods to functions. Updated the related import statements and function invokation lines in backend/domain/services files, including tags_service.py, difficulty_distribution.py, and abandoned_problem_service.py.

…ssions.py files to correctly call the functions in backend/domain/services/base.py. Change due to shift from BaseMetricService class with static methods to standalone functions.

deyna256

You did a great job! But pulling private functions out of the services package seems very strange to me. I have an idea about refactoring the private functions - let's make an abstraction which will encapsulate a bunch of Problem objects. It can have both of our private methods to do some filtering with the Problem objects it has. What do you think?

sonnenco · 2026-02-17T00:07:29Z

Hello @deyna256 - thank you for the feedback. I can adjust, but would like to confirm my understanding of your proposed solution before I make further changes:

Current

Private functions declared in base.py
Private functions are imported for use into other files in the Services package and test suite

Future

Create a class in base.py that contains both of the non-static methods and initializes attributes for _submissions and _problems
Methods can access these attributes to perform deduplication and filtering capabilities within the class
Use the new class in the other files in the Services package and test suite

Please let me know if I've missed anything or misunderstood.

deyna256 · 2026-02-17T09:15:15Z

@sonnenco exactly! let's do it!

…es + test files to accommodate.

deyna256

@sonnenco you did a great job! Just fix some issues and we are good to merge it!

deyna256 · 2026-02-18T08:57:46Z


-    @staticmethod
-    def _deduplicate_problems(submissions: List[Submission]) -> List[Submission]:
+    def _deduplicate_problems(self) -> List[Submission]:


In Python, prefixing a method name with an underscore (_) is a convention used to signal that the method is intended to be private (i.e., not part of the public API). But in our case methods: _deduplicate_problems and _filter_successful_submissions are public API of SubmissionProcessor class, rename them

deyna256 · 2026-02-18T09:02:14Z


 from backend.domain.models.codeforces import Submission

+class SubmissionProcessor:


Why do we name the class like this? A developer cannot understand its meaning if they don't read its methods, so we should rename it.

deyna256 · 2026-02-18T09:08:24Z

@sonnenco also you need to fix the checks: Backend Tests, Backend Type Checker

…m private to public functions. Retain original submissions and perform operations on working copy.

…me in ARCHITECTURE.md. Removed class reference in __init__.py as SubmissionCollection is a helper.

sonnenco · 2026-02-19T00:38:16Z

backend testing

just test passes required test coverage, very similar results to main branch
just check states "all checks passed" (runs lint + typecheck)

frontend testing

just typecheck states "npx tsc --noEmit"
just lint doesn't come back with anything

Docker container builds fine locally (just build) and is accessible from http://localhost:3000 (just up).

Awaiting validation from workflows to double check results.

sonnenco · 2026-02-19T00:39:54Z

Thank you for your patience @deyna256. I've addressed the feedback as best I can. Re-requesting review.

…dification within class methods. Updated method calls in associated backend services and testing files.

sonnenco added 2 commits February 15, 2026 12:41

Updated test_deduplicate_problems.py and test_filter_successful_submi…

e54b509

…ssions.py files to correctly call the functions in backend/domain/services/base.py. Change due to shift from BaseMetricService class with static methods to standalone functions.

deyna256 requested changes Feb 16, 2026

View reviewed changes

deyna256 assigned sonnenco Feb 16, 2026

sonnenco added 2 commits February 17, 2026 20:07

Created class SubmissionProcessor in base.py and updated services fil…

107f9a4

…es + test files to accommodate.

Merge branch 'main' into 17

83e9abd

sonnenco requested a review from deyna256 February 18, 2026 01:26

deyna256 requested changes Feb 18, 2026

View reviewed changes

sonnenco added 3 commits February 18, 2026 18:52

Renamed class to SubmissionCollection for better clarity. Shifted fro…

54bf387

…m private to public functions. Retain original submissions and perform operations on working copy.

Updated object references in service and test files. Updated class na…

9955170

…me in ARCHITECTURE.md. Removed class reference in __init__.py as SubmissionCollection is a helper.

Merge branch 'main' into 17

185bcac

sonnenco requested a review from deyna256 February 19, 2026 00:39

deyna256 requested changes Feb 19, 2026

View reviewed changes

Comment thread backend/domain/services/base.py Outdated

sonnenco added 2 commits February 21, 2026 07:22

Never overwrite self._submissions. Create copies of collection for mo…

f47662a

…dification within class methods. Updated method calls in associated backend services and testing files.

Merge remote-tracking branch 'origin/main' into 17

2aff7b6

sonnenco requested a review from deyna256 February 20, 2026 22:33

Navigation Menu

Refactor BaseMetricService (#17) - #37

Refactor BaseMetricService (#17)#37
sonnenco wants to merge 9 commits into
deyna256:mainfrom
sonnenco:17

sonnenco commented Feb 15, 2026

Uh oh!

deyna256 left a comment

Uh oh!

sonnenco commented Feb 17, 2026

Uh oh!

deyna256 commented Feb 17, 2026

Uh oh!

deyna256 left a comment

Uh oh!

deyna256 Feb 18, 2026

Uh oh!

deyna256 Feb 18, 2026

Uh oh!

Uh oh!

deyna256 commented Feb 18, 2026

Uh oh!

sonnenco commented Feb 19, 2026

Uh oh!

sonnenco commented Feb 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		from backend.domain.models.codeforces import Submission

		class SubmissionProcessor:

Conversation

sonnenco commented Feb 15, 2026

Issue

Changes

Testing

Uh oh!

deyna256 left a comment

Choose a reason for hiding this comment

Uh oh!

sonnenco commented Feb 17, 2026

Uh oh!

deyna256 commented Feb 17, 2026

Uh oh!

deyna256 left a comment

Choose a reason for hiding this comment

Uh oh!

deyna256 Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

deyna256 Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

deyna256 commented Feb 18, 2026

Uh oh!

sonnenco commented Feb 19, 2026

Uh oh!

sonnenco commented Feb 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants